pkg/reconciler/customresourcesgate/reconciler_test.go (1)

567-647: Good test coverage for GroupPredicate — one question about event type coverage.

Thanks for the comprehensive test cases! The table-driven structure follows Crossplane conventions nicely, and the test cases cover the key scenarios well:

• ✅ CRD group present in filter
• ✅ CRD group absent from filter
• ✅ Empty groups list
• ✅ Non-CRD object handling

One curiosity: I noticed the test only exercises got.Generic(ev). Since predicate.NewPredicateFuncs applies the same function to all event types (Create, Update, Delete, Generic), this should be fine in practice. However, would you consider adding at least one test case that exercises Create or Update events to document that the predicate is applied consistently across event types? This could help future maintainers understand that the filtering applies to all watch events.

Totally optional — what are your thoughts?

💡 Example of testing Create event (optional)

"CRDGroupPresentCreateEvent": {
    reason: "CRD is present in the specified groups for Create event",
    args: args{
        groups: []string{"example.com"},
        obj: &apiextensionsv1.CustomResourceDefinition{
            Spec: apiextensionsv1.CustomResourceDefinitionSpec{
                Group: "example.com",
            },
        },
    },
    want: want{
        match: true,
    },
},

Then in the test loop, you could add:

createEv := event.CreateEvent{Object: tc.args.obj}
if got.Create(createEv) != tc.want.match {
    // ...
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/reconciler/customresourcesgate/reconciler_test.go` around lines 567 -
647, Add an extra assertion in TestGroupPredicate to exercise at least one
non-Generic event (e.g., Create or Update) so the predicate behavior across
event types is documented; locate the GroupPredicate(...) call and the existing
ev := event.GenericEvent{Object: tc.args.obj} and add a corresponding createEv
:= event.CreateEvent{Object: tc.args.obj} (or updateEv) and assert
got.Create(createEv) (or got.Update(updateEv)) equals tc.want.match, failing the
test with the same cmp.Diff-based message if it does not match.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

pkg/reconciler/customresourcesgate/reconciler_test.go (1)

567-647: Test the predicate across all event types (Create, Update, Delete, Generic) to ensure consistent filtering behavior.

The GroupPredicate implementation uses predicate.NewPredicateFuncs(), which applies the same filtering logic to all event types during watch operations. Currently, the test covers only GenericEvent. Consider adding equivalent test cases for Create, Update, and Delete events—this will pin down behavior across all code paths the predicate actually handles in the controller.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/reconciler/customresourcesgate/reconciler_test.go` around lines 567 -
647, Extend TestGroupPredicate to exercise GroupPredicate across all event
types: for each case call the predicate's Create, Update, Delete and Generic
handlers (use event.CreateEvent, event.UpdateEvent, event.DeleteEvent,
event.GenericEvent) with appropriate Objects (for UpdateEvent provide OldObject
and Object) and assert the boolean result equals tc.want.match; locate the test
loop in TestGroupPredicate and duplicate the current GenericEvent check into
checks for got.Create, got.Update, and got.Delete so the predicate behavior is
verified for all event paths.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro